ui/src/components/ui/MetricCard.tsx (1)

15-18: Add screen-reader text for trend indicators.

The ▲/▼ glyphs are visual-only; add an SR label and mark the glyphs aria-hidden for accessibility.

Proposed fix

       <div className="flex items-baseline gap-1 mt-1">
         <p className={cn('text-2xl font-bold font-mono', valueClassName || 'text-card-text')}>{value}</p>
-        {trend === 'up' && <span className="text-urgency-green text-sm">&#9650;</span>}
-        {trend === 'down' && <span className="text-urgency-red text-sm">&#9660;</span>}
+        {trend && (
+          <span className="sr-only">
+            {trend === 'up' ? 'Trending up' : 'Trending down'}
+          </span>
+        )}
+        {trend === 'up' && <span aria-hidden="true" className="text-urgency-green text-sm">&#9650;</span>}
+        {trend === 'down' && <span aria-hidden="true" className="text-urgency-red text-sm">&#9660;</span>}
       </div>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/components/ui/MetricCard.tsx` around lines 15 - 18, The trend glyphs
in MetricCard are currently visual-only; update the two spans that render the
▲/▼ (the JSX that checks trend === 'up' and trend === 'down') to mark the glyph
elements aria-hidden="true" and include an adjacent screen-reader-only text node
(e.g., a span with an sr-only class) that reads an accessible label like
"Increasing" for up and "Decreasing" for down so assistive tech receives the
meaning while the glyph remains visual-only.

ui/src/lib/focus-mode.tsx (1)

13-24: Harden localStorage access for non-browser or blocked storage.

localStorage access can throw in SSR/test environments or restricted storage modes. Guard and fail safe to default ON.

Proposed fix

 export function FocusModeProvider({ children }: { children: ReactNode }) {
   const [focusMode, setFocusMode] = useState(() => {
-    const saved = localStorage.getItem('chittycommand_focus_mode');
-    return saved !== null ? saved === 'true' : true; // default ON
+    if (typeof window === 'undefined') return true;
+    try {
+      const saved = window.localStorage.getItem('chittycommand_focus_mode');
+      return saved !== null ? saved === 'true' : true; // default ON
+    } catch {
+      return true;
+    }
   });

   const toggleFocusMode = useCallback(() => {
     setFocusMode((prev) => {
       const next = !prev;
-      localStorage.setItem('chittycommand_focus_mode', String(next));
+      try {
+        window.localStorage.setItem('chittycommand_focus_mode', String(next));
+      } catch {
+        // ignore write failures (private mode / quota / disabled storage)
+      }
       return next;
     });
   }, []);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/lib/focus-mode.tsx` around lines 13 - 24, FocusModeProvider currently
accesses localStorage directly in the useState initializer and in
toggleFocusMode which can throw in SSR/tests or when storage is blocked; wrap
reads/writes in safe guards (check typeof window !== 'undefined' and wrap
localStorage.getItem/localStorage.setItem calls in try/catch) and fall back to
default true on error; update the useState initializer (FocusModeProvider) to
catch exceptions and return true if storage is unavailable, and update
toggleFocusMode to attempt to persist inside try/catch without throwing if
setItem fails.

ui/src/components/ui/ProgressDots.tsx (1)

9-23: Clamp progress values to avoid RangeError and overfill.

Array.from({ length: total }) throws if total is negative, and completed > total overfills the visual state. Consider clamping defensively.

Proposed fix

 export function ProgressDots({ completed, total, className }: ProgressDotsProps) {
+  const safeTotal = Math.max(0, total);
+  const safeCompleted = Math.min(Math.max(0, completed), safeTotal);
   return (
     <div className={cn('flex items-center gap-1', className)}>
-      {Array.from({ length: total }, (_, i) => (
+      {Array.from({ length: safeTotal }, (_, i) => (
         <span
           key={i}
           className={cn(
             'w-2 h-2 rounded-full',
-            i < completed ? 'bg-urgency-green' : 'bg-card-border',
+            i < safeCompleted ? 'bg-urgency-green' : 'bg-card-border',
           )}
         />
       ))}
       <span className="text-xs text-card-muted ml-1">
-        {completed}/{total}
+        {safeCompleted}/{safeTotal}
       </span>
     </div>
   );
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/components/ui/ProgressDots.tsx` around lines 9 - 23, The ProgressDots
component should defensively clamp the input props so negative or non-finite
totals and out-of-range completed values cannot cause a RangeError or visual
overfill: inside ProgressDots compute a safeTotal (e.g., Math.max(0,
Math.floor(total || 0)) or ensure Number.isFinite) and a safeCompleted clamped
between 0 and safeTotal (e.g., Math.min(Math.max(0, Math.floor(completed || 0)),
safeTotal)), then use safeTotal for Array.from length and safeCompleted for
deciding the filled dot class and the displayed {completed}/{total} values (or
display the clamped values) to prevent errors and overfill.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ui/src/pages/Disputes.tsx (1)

40-46: Clear panel list immediately when switching disputes to avoid stale render.

Same issue as noted before: without clearing the list before the fetch, the previous dispute’s items can flash until the new response arrives.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Disputes.tsx` around lines 40 - 46, When switching disputes,
clear the old panel lists immediately to avoid flashing stale items: after
calling setExpandedId(disputeId), setActivePanel(panel) and setPanelError(null),
call setCorrespondenceList([]) and setDocumentList([]) (or at least clear the
one not needed) before awaiting api.getDispute(disputeId) so the UI renders an
empty list while the new detail is fetched; update the logic around
setCorrespondenceList and setDocumentList in the Disputes.tsx handler that calls
api.getDispute to implement this.

ui/src/components/ui/UrgencyBorder.ts (1)

1-6: NaN/Infinity values silently map to 'green'.

This was flagged in a prior review. NaN >= 70 and NaN >= 40 are both false, so non-finite scores fall through to 'green', which is misleading.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/components/ui/UrgencyBorder.ts` around lines 1 - 6, The function
urgencyLevel currently treats NaN/Infinity as valid and falls through to
'green'; update urgencyLevel to first verify the input is a finite number (e.g.,
typeof score === 'number' && Number.isFinite(score') or Number.isFinite(score))
and return null for non-finite values (including NaN and ±Infinity) before
running the >= 70 / >= 40 checks so only finite numeric scores map to 'red',
'amber', or 'green'.

ui/src/pages/Upload.tsx (1)

89-94: ActionButton loading prop overrides the custom "Uploading…" label.

Same issue flagged in a prior review. ActionButton renders "Working..." when loading={true}, so your 'Uploading...' label is dead code.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Upload.tsx` around lines 89 - 94, The ActionButton's built-in
loading behavior overrides your custom 'Uploading...' label; in Upload.tsx stop
passing the loading prop and instead control disabled state so your custom label
is shown: remove loading={uploading} from the <ActionButton ... /> call and
change disabled={!pendingCount} to disabled={uploading || !pendingCount}; keep
label={uploading ? 'Uploading...' : `Upload ${pendingCount} File${pendingCount
!== 1 ? 's' : ''}`} and leave handleUploadAll as-is (or alternatively update
ActionButton to accept a loadingLabel prop if you prefer keeping a spinner).

ui/src/pages/Settings.tsx (2)

228-241: BridgeSyncButton duplicates ActionButton — use the shared component instead

The rest of the page already uses ActionButton (lines 107-112) for interactive sync controls. BridgeSyncButton re-implements a button with loading state manually, diverging from the design system.

♻️ Proposed refactor

-function BridgeSyncButton({ label, syncing, onClick }: { label: string; syncing: boolean; onClick: () => void }) {
-  return (
-    <button
-      onClick={onClick}
-      disabled={syncing}
-      className="flex items-center justify-between px-3 py-2 text-sm bg-card-hover border border-card-border rounded-lg hover:border-chitty-500 disabled:opacity-50 transition-colors"
-    >
-      <span className="text-card-text">{label}</span>
-      <span className={`text-xs px-2 py-0.5 rounded-full ${syncing ? 'bg-amber-100 text-amber-700' : 'bg-gray-100 text-gray-600'}`}>
-        {syncing ? 'Syncing...' : 'Sync'}
-      </span>
-    </button>
-  );
-}

Replace each <BridgeSyncButton ... /> usage with:

<ActionButton
  label={label}
  onClick={onClick}
  loading={syncing}
  className="w-full text-sm"
/>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Settings.tsx` around lines 228 - 241, BridgeSyncButton
duplicates the existing ActionButton design system; replace BridgeSyncButton
usages with the shared ActionButton component and remove the duplicate
component. Specifically, where BridgeSyncButton is used pass label -> label,
onClick -> onClick, syncing -> loading to ActionButton (e.g., <ActionButton
label={label} onClick={onClick} loading={syncing} ...>), update any className
props as needed (for example className="w-full text-sm"), and delete the
BridgeSyncButton function to avoid duplication.

---

218-226: SyncStatusBadge uses hardcoded Tailwind color literals instead of design tokens

The new design system introduces urgency/card tokens, but SyncStatusBadge maps 'completed', 'started', and 'error' to raw bg-green-100 text-green-700 etc. Consider aligning to text-urgency-green / text-urgency-red tokens for consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Settings.tsx` around lines 218 - 226, SyncStatusBadge currently
uses hardcoded Tailwind color classes; update the colors mapping inside the
SyncStatusBadge function to use the new design tokens (e.g., replace
'bg-green-100 text-green-700' / 'bg-amber-100 text-amber-700' / 'bg-red-100
text-red-700' with the corresponding token classes such as 'bg-urgency-green
text-urgency-green', 'bg-urgency-amber text-urgency-amber', 'bg-urgency-red
text-urgency-red' or whatever token names your design system defines), ensure
the fallback also uses the neutral design tokens (e.g., 'bg-utility-bg
text-utility-fg'), and keep the rest of the JSX in function SyncStatusBadge
unchanged so the badge styling is consistent with the new design system.

ui/src/components/ui/UrgencyBorder.ts (1)

1-6: Naming collision with urgencyLevel in ui/src/lib/urgency.ts.

There's another urgencyLevel export in ui/src/lib/urgency.ts (lines 58–63) with different thresholds (>=50 → 'high', >=30 → 'medium') and a different return type ('critical' | 'high' | 'medium' | 'low'). Having two identically named functions with divergent semantics is a maintenance trap—consumers can accidentally import the wrong one.

Consider renaming one (e.g., urgencyColor here since it returns color names) or consolidating into a single module.

#!/bin/bash
# Check which files import from each urgencyLevel source
echo "=== Imports from components/ui/UrgencyBorder ==="
rg -n "from.*UrgencyBorder" --type=ts --type=tsx -g '!node_modules'
rg -n "from.*UrgencyBorder" -g '*.tsx' -g '*.ts' -g '!node_modules'

echo ""
echo "=== Imports from lib/urgency ==="
rg -n "from.*lib/urgency" -g '*.tsx' -g '*.ts' -g '!node_modules'

echo ""
echo "=== All urgencyLevel references ==="
rg -nP '\burgencyLevel\b' -g '*.tsx' -g '*.ts' -g '!node_modules'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/components/ui/UrgencyBorder.ts` around lines 1 - 6, There is a naming
collision between the urgencyLevel exported from UrgencyBorder.ts and another
urgencyLevel in ui/src/lib/urgency.ts; rename the function in UrgencyBorder.ts
to a distinct, descriptive name (e.g., urgencyColor) and update its export,
signature reference (export function urgencyColor(score: number | null):
'red'|'amber'|'green'|null), and all imports across the codebase that currently
import urgencyLevel from components/ui/UrgencyBorder to import urgencyColor
instead; alternatively, if you prefer consolidation, move the color-mapping
logic into ui/src/lib/urgency.ts under a new export name (urgencyColor) and
replace the UrgencyBorder.ts usage to import that single function so there's one
canonical API.

ui/src/pages/Recommendations.tsx (1)

70-86: priorityColors and typeColors are constant dictionaries re-created every render.

Move them to module scope to avoid unnecessary allocations.

Move to module level

+const priorityColors: Record<number, string> = {
+  1: 'bg-red-100 text-red-700',
+  2: 'bg-orange-100 text-orange-700',
+  3: 'bg-amber-100 text-amber-700',
+  4: 'bg-blue-100 text-blue-700',
+  5: 'bg-gray-100 text-gray-700',
+};
+
+const typeColors: Record<string, string> = {
+  payment: 'bg-green-50 text-green-700 border-green-200',
+  negotiate: 'bg-purple-50 text-purple-700 border-purple-200',
+  defer: 'bg-gray-50 text-gray-600 border-gray-200',
+  dispute: 'bg-orange-50 text-orange-700 border-orange-200',
+  legal: 'bg-red-50 text-red-700 border-red-200',
+  warning: 'bg-amber-50 text-amber-700 border-amber-200',
+  strategy: 'bg-blue-50 text-blue-700 border-blue-200',
+};
+
+const actionLabel = (type: string | null): string => {
+  ...
+};
+
 export function Recommendations() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Recommendations.tsx` around lines 70 - 86, priorityColors and
typeColors are being re-created on every render inside the component; move both
constant dictionaries to module scope (outside the component function) so they
are only allocated once. Locate the definitions of priorityColors and typeColors
in Recommendations.tsx, cut them from inside the component, and declare them at
top-level with the same types (Record<number,string> and Record<string,string>)
so the component can reference them without reallocation. Ensure imports/types
used in the file still resolve after moving them.

ui/src/pages/Bills.tsx (1)

14-18: No loading state shown while obligations are being fetched.

Unlike Recommendations.tsx which has a loading state and shows a loading message, Bills renders an empty list during the initial fetch. Consider adding a loading indicator for better UX, especially on slower connections.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/Bills.tsx` around lines 14 - 18, The Bills component's useEffect
calls api.getObligations without a loading flag, causing the UI to show an empty
list during fetch; add a loading state (e.g., const [loading, setLoading] =
useState(false)) in the Bills component, setLoading(true) before calling
api.getObligations in the useEffect, then setLoading(false) in a finally block
(or after both setObligations and setError) to ensure it's cleared on success or
error, and update the component render to show a loading indicator/message when
loading is true (mirror the pattern used in Recommendations.tsx); key symbols:
useEffect, api.getObligations, setObligations, setError, loading, setLoading.

ui/src/pages/CashFlow.tsx (1)

146-165: Use a stable, unique key derived from the underlying data instead of array index.

Index keys cause React to reuse DOM elements when the filtered list changes, leading to state/animation issues. The suggested composite key from date and values may work in practice, but consider exposing the database id field in the CashflowProjection interface and using that instead for guaranteed uniqueness and stability.

♻️ Suggested change

-                {chartData.filter((e) => e.outflow > 0).slice(0, 15).map((entry, i) => (
-                  <tr key={i} className="border-b border-card-border/50 last:border-0">
+                {chartData.filter((e) => e.outflow > 0).slice(0, 15).map((entry) => (
+                  <tr key={`${entry.date}-${entry.outflow}-${entry.balance}`} className="border-b border-card-border/50 last:border-0">

Or, add id to the CashflowProjection interface in ui/src/lib/api.ts and use key={entry.id} for true stability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/CashFlow.tsx` around lines 146 - 165, The current map in
CashFlow.tsx uses the array index as the React key for chartData.map, which is
unstable; replace key={i} with a stable unique identifier from the data
(preferably entry.id) by adding an id to the CashflowProjection interface in
ui/src/lib/api.ts and exposing it from the backend, then use key={entry.id}; if
you cannot add an id immediately, use a deterministic composite key such as
`${entry.date}-${entry.outflow}-${entry.balance}` for the map over
chartData.filter(...).slice(...).map to avoid index-based reuse issues.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

ui/src/pages/CashFlow.tsx (1)

70-72: ⚠️ Potential issue | 🟠 Major

Unresolved: date label rolls back one day in US timezones.

new Date('YYYY-MM-DD') parses as UTC midnight; without timeZone: 'UTC' in toLocaleDateString, US locales display the previous calendar day. The same label is then used as the table row key at line 158, so a misformatted date propagates to both the chart axis and the table.

🐛 Proposed fix

  const chartData = projections.map((p) => ({
-   date: new Date(p.projection_date).toLocaleDateString('en-US', { month: 'short', day: 'numeric' }),
+   date: new Date(p.projection_date).toLocaleDateString('en-US', {
+     month: 'short',
+     day: 'numeric',
+     timeZone: 'UTC',
+   }),
+   projectionDate: p.projection_date,   // preserve for stable key usage
    balance: parseFloat(p.projected_balance),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/CashFlow.tsx` around lines 70 - 72, The date labels created in
chartData via projections.map (using new
Date(p.projection_date).toLocaleDateString(...)) roll back one day in US
timezones; update the formatting to force UTC (e.g., toLocaleDateString('en-US',
{ month: 'short', day: 'numeric', timeZone: 'UTC' })) or alternatively parse
with an explicit UTC timestamp (e.g., append 'T00:00:00Z' to p.projection_date)
so the chart axis matches the intended calendar date; also ensure the same
normalized label or the raw p.projection_date is used as the table row key (the
code that sets the key for the table row should reference the same formatted
value or use p.projection_date) to avoid mismatched keys between the chart and
table.

package.json (1)

26-26: Add a test script to expose vitest via npm test.

The PR adds 59 urgency-scoring tests and wires vitest as a devDependency, but no test (or test:unit) script is defined in scripts. Without it, running the test suite requires knowing to invoke npx vitest directly rather than the conventional npm test.

Version 4.0.18 is confirmed as the current latest on npm, so the pin is valid. Vitest 4.0 includes breaking changes relative to prior major versions, but since this is a fresh addition rather than an upgrade, that's not a concern here.

🛠️ Proposed addition of a `test` script

     "ui:dev": "cd ui && vite dev",
     "ui:build": "cd ui && vite build",
-    "ui:preview": "cd ui && vite preview"
+    "ui:preview": "cd ui && vite preview",
+    "test": "vitest run",
+    "test:watch": "vitest"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 26, package.json lacks a "test" script exposing vitest;
add a scripts entry (e.g., "test": "vitest") so running npm test invokes the new
vitest devDependency. Update the "scripts" object in package.json to include the
"test" key (or "test:unit" if you prefer) referencing the installed "vitest" so
contributors can run tests via npm test rather than npx vitest.

ui/src/pages/CashFlow.tsx (3)

73-75: parseFloat still used here while the PR standardised on Number() elsewhere.

parseFloat("12.5abc") silently returns 12.5; Number("12.5abc") returns NaN, which is the correct signal for a malformed API value. The same applies to line 188 (parseFloat(ob.amount_due || ...)).

♻️ Proposed refactor

-   balance: parseFloat(p.projected_balance),
-   inflow: parseFloat(p.projected_inflow),
-   outflow: parseFloat(p.projected_outflow),
+   balance: Number(p.projected_balance),
+   inflow: Number(p.projected_inflow),
+   outflow: Number(p.projected_outflow),

And on line 188:

-  <span ...>{formatCurrency(parseFloat(ob.amount_due || ob.amount_minimum || '0'))}</span>
+  <span ...>{formatCurrency(Number(ob.amount_due ?? ob.amount_minimum ?? '0'))}</span>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/CashFlow.tsx` around lines 73 - 75, Replace the parseFloat calls
in CashFlow.tsx with Number so malformed strings yield NaN instead of silently
parsing; specifically change the conversions for the object properties balance,
inflow, and outflow (currently using parseFloat(p.projected_balance /
projected_inflow / projected_outflow)) to use Number(p.projected_balance) etc.,
and likewise replace parseFloat(ob.amount_due || ...) with Number(ob.amount_due
|| ...) where ob.amount_due is processed; leave surrounding logic unchanged so
invalid API values are surfaced as NaN.

---

157-158: Use projectionDate (the raw ISO date string) as the table row key instead of the formatted label.

Formatted strings like "Feb 10" are not globally unique (they repeat across years) and won't survive a dataset that spans year boundaries. The raw projection_date value is already unique and was noted as a "stable keys" fix target in this PR's commits.

♻️ Proposed fix

Expose the raw date in chartData (also useful for the timezone fix above):

  const chartData = projections.map((p) => ({
    date: new Date(p.projection_date).toLocaleDateString(...),
+   projectionDate: p.projection_date,
    balance: ...,

Then use it as the key:

- {chartData.filter((e) => e.outflow > 0).slice(0, 15).map((entry) => (
-   <tr key={entry.date} ...>
+ {chartData.filter((e) => e.outflow > 0).slice(0, 15).map((entry) => (
+   <tr key={entry.projectionDate} ...>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/CashFlow.tsx` around lines 157 - 158, The table row key uses the
formatted label (entry.date) which is not globally unique across years; update
the chartData shape to include the raw ISO date (named projection_date or
projectionDate) and change the row key in the map inside CashFlow.tsx to use
that raw value (e.g., key={entry.projectionDate} or key={entry.projection_date})
instead of entry.date so keys remain stable across year boundaries; ensure
wherever chartData is assembled you expose projectionDate/projection_date so the
map callback has access to it.

---

132-132: YAxis formatter places $ before the sign for negative values ($-5k) and rounds values under $500 to $0k.

Both are cosmetic but can mislead on a cash-flow chart where negative balances and sub-$500 granularity matter.

♻️ Proposed fix

- tickFormatter={(v) => `$${(v / 1000).toFixed(0)}k`}
+ tickFormatter={(v) => {
+   const abs = Math.abs(v);
+   const formatted = abs >= 1000 ? `$${(abs / 1000).toFixed(0)}k` : `$${abs}`;
+   return v < 0 ? `-${formatted}` : formatted;
+ }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ui/src/pages/CashFlow.tsx` at line 132, The YAxis tickFormatter currently
formats values like `$-5k` and collapses values under $500 to `$0k`; update the
tickFormatter used on the YAxis so the minus sign appears before the dollar sign
and small amounts keep dollar granularity instead of rounding to `0k`.
Specifically, in the YAxis tickFormatter function referenced in the file
(tickFormatter on YAxis) compute a sign prefix ( '-' for negative else '' ), use
Math.abs(v) for magnitude, and if abs(value) < 1000 return a dollar-formatted
string with dollars (e.g., "-$500" or "$250") otherwise format in thousands with
`k` (e.g., "-$5k" or "$12k") so negatives show as "-$X" and sub-$1000 values are
not rounded to `0k`.

src/lib/urgency.test.ts (1)

316-338: Weak assertions in invalid-input tests won't catch scoring regressions.

The invalid date and empty string tests (lines 317–328) only assert that the score is within [0, 100]. Any score in that range would pass, including one where time-pressure was incorrectly applied. Both cases should fall through to category-weight-only, giving a deterministic score of 15. Consider pinning to that value.

♻️ Proposed fix

    it('invalid date string returns 0 (fails gracefully)', () => {
      const score = computeUrgencyScore({ ...base, due_date: 'not-a-date' });
-     // Should handle NaN gracefully, not crash
-     expect(score).toBeGreaterThanOrEqual(0);
-     expect(score).toBeLessThanOrEqual(100);
+     // NaN date → skip time pressure → category weight only (utility = 15)
+     expect(score).toBe(15);
    });

    it('empty date string returns category weight only (no time pressure)', () => {
      const score = computeUrgencyScore({ ...base, due_date: '' });
-     expect(score).toBeGreaterThanOrEqual(0);
-     expect(score).toBeLessThanOrEqual(100);
+     expect(score).toBe(15);
    });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/urgency.test.ts` around lines 316 - 338, The tests for invalid date
inputs use weak range checks; update the two cases in the invalid-inputs block
so that computeUrgencyScore returns the deterministic category-weight-only value
(15) instead of merely asserting 0–100. Specifically, change the assertions for
computeUrgencyScore called with due_date: 'not-a-date' and due_date: '' to
expect( score ).toBe(15), keeping the existing NaN and negative late_fee cases
as-is; locate these checks around the computeUrgencyScore calls in the failing
test block to make the replacements.

src/lib/urgency.ts (1)

56-59: lateFee can hold a negative value despite the "guard against NaN/negative" comment.

isFinite(-10) is true, so a negative late_fee passes through as-is rather than being normalised to 0. The subsequent > 0/> 25/> 50 checks happen to produce the correct result (no bonus added), but the variable itself carries a misleading value that diverges from the stated intent.

♻️ Proposed fix

-  const lateFee = obligation.late_fee != null && isFinite(obligation.late_fee) ? obligation.late_fee : 0;
+  const rawFee = obligation.late_fee;
+  const lateFee = rawFee != null && isFinite(rawFee) && rawFee > 0 ? rawFee : 0;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/urgency.ts` around lines 56 - 59, The lateFee variable currently
allows negative numbers because isFinite only filters NaN/infinite; change the
normalization of obligation.late_fee so negative values are clamped to 0 (e.g.,
set lateFee = Math.max(0, parsedValue)) before the scoring logic that modifies
score (references: lateFee, obligation.late_fee, score in the urgency
computation); ensure the replacement still guards against non-finite values and
falls back to 0 when invalid.

src/lib/validators.ts (2)

171-181: dateString validates format only — semantically invalid dates like 2024-02-30 pass.

The regex /^\d{4}-\d{2}-\d{2}$/ confirms shape but not calendar validity. For query params this is low-risk (an out-of-range date will simply match nothing), but a .refine() check would catch client bugs earlier.

♻️ Optional semantic validation

-const dateString = z.string().regex(/^\d{4}-\d{2}-\d{2}$/, 'Must be YYYY-MM-DD');
+const dateString = z
+  .string()
+  .regex(/^\d{4}-\d{2}-\d{2}$/, 'Must be YYYY-MM-DD')
+  .refine((s) => !isNaN(Date.parse(s)), 'Must be a valid calendar date');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/validators.ts` around lines 171 - 181, The dateString regex currently
only enforces YYYY-MM-DD shape but allows invalid calendar dates; update the
dateString declaration to add a semantic .refine() that parses the captured
year/month/day (from the string) and constructs a Date (or uses Date.UTC) then
verifies the resulting date matches the original year, month, and day and is not
NaN, returning a clear error like "Invalid calendar date"; keep the existing
regex and error text for format, then reference dateString in
obligationCalendarQuerySchema so start/end retain validation.

---

183-188: disputeQuerySchema and recommendationQuerySchema accept unconstrained status strings, unlike obligationQuerySchema.

obligationQuerySchema (lines 173–176) constrains status to a typed enum, returning a 400 for unrecognised values. The dispute and recommendation schemas accept any string up to 50 chars, so a typo like status=openn passes validation and silently returns an empty dataset rather than a 400. Adopting enums here makes the API behaviour consistent and self-documenting.

♻️ Proposed fix

 export const disputeQuerySchema = z.object({
-  status: z.string().max(50).optional(),
+  status: z.enum(['open', 'resolved', 'dismissed']).optional(),
 });

 export const recommendationQuerySchema = z.object({
-  status: z.string().max(50).optional(),
+  status: z.enum(['active', 'completed', 'dismissed']).optional(),
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/validators.ts` around lines 183 - 188, The disputeQuerySchema and
recommendationQuerySchema currently accept any status string; change them to
reuse the same enum/type used by obligationQuerySchema so invalid statuses fail
validation. Replace the z.string().max(50).optional() for status in
disputeQuerySchema and recommendationQuerySchema with the same Zod
enum/nativeEnum used by obligationQuerySchema (import/reuse the enum symbol used
there) and keep .optional(); ensure the schemas return a 400 on unrecognised
enum values by using the identical enum type definition as
obligationQuerySchema.

src/routes/obligations.ts (2)

160-163: Hoist todayUtc above the loop — it's date-constant and re-created per iteration.

now and todayUtc don't depend on ob and are recomputed every iteration. Move them before the for loop for clarity and to guarantee a single consistent "today" for the entire batch.

Proposed fix

+ const now = new Date();
+ const todayUtc = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()));
  for (const ob of obligations) {
    const score = computeUrgencyScore({
      ...
    });
    const dueDate = new Date(ob.due_date + 'T00:00:00Z');
-   const now = new Date();
-   const todayUtc = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()));
    const newStatus = dueDate < todayUtc && ob.status === 'pending' ? 'overdue' : ob.status;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/routes/obligations.ts` around lines 160 - 163, The calculation of now and
todayUtc is done inside the per-obligation loop (they're recomputed each
iteration); move the Date construction for now and the Date.UTC-based todayUtc
out of the loop so they are computed once before iterating over obligations,
then keep using todayUtc inside the loop when computing dueDate and newStatus
(the expressions involving dueDate, ob.status and newStatus remain the same).
This guarantees a single consistent "today" for all obligations and avoids
repeated allocations.

---

157-157: Avoid double parseFloat — extract to a local.

Proposed fix

-      late_fee: ob.late_fee ? (isFinite(parseFloat(ob.late_fee)) ? parseFloat(ob.late_fee) : null) : null,
+      late_fee: (() => { const v = ob.late_fee ? parseFloat(ob.late_fee) : NaN; return isFinite(v) ? v : null; })(),

Or more readably, extract a small helper used here and in the PATCH handler:

const safeLateFee = (val: unknown): number | null => {
  if (!val) return null;
  const n = parseFloat(String(val));
  return isFinite(n) ? n : null;
};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/routes/obligations.ts` at line 157, The late_fee mapping currently calls
parseFloat twice for ob.late_fee; extract a small helper (e.g., safeLateFee)
that takes an unknown, coerces to string, parses once, and returns a finite
number or null, then replace the inline expression in the late_fee property and
the PATCH handler to call safeLateFee(ob.late_fee) to avoid double parsing and
improve readability.